fix: Error when resolving raw external array#2408
Conversation
|
pkg.pr.new packages benchmark commit |
📊 Bundle Size Comparison
👀 Notable resultsStatic test results:No major changes. Dynamic test results:No major changes. 📋 All resultsClick to reveal the results table (353 entries).
If you wish to run a comparison for other, slower bundlers, run the 'Tree-shake test' from the GitHub Actions menu. |
Resolution Time Benchmark---
config:
themeVariables:
xyChart:
plotColorPalette: "#E63946, #3B82F6, #059669"
---
xychart
title "Random Branching (🔴 PR | 🔵 main | 🟢 release)"
x-axis "max depth" [1, 2, 3, 4, 5, 6, 7, 8]
y-axis "time (ms)"
line [0.97, 1.95, 4.00, 5.91, 7.13, 10.02, 21.61, 21.76]
line [0.95, 1.88, 4.11, 6.23, 7.08, 11.03, 21.08, 21.09]
line [0.95, 1.89, 3.97, 5.91, 7.48, 11.20, 20.28, 21.67]
---
config:
themeVariables:
xyChart:
plotColorPalette: "#E63946, #3B82F6, #059669"
---
xychart
title "Linear Recursion (🔴 PR | 🔵 main | 🟢 release)"
x-axis "max depth" [1, 2, 3, 4, 5, 6, 7, 8]
y-axis "time (ms)"
line [0.29, 0.54, 0.68, 0.89, 1.17, 1.23, 1.43, 1.67]
line [0.35, 0.55, 0.71, 0.82, 1.13, 1.18, 1.42, 1.59]
line [0.32, 0.55, 0.71, 0.80, 1.07, 1.18, 1.46, 1.58]
---
config:
themeVariables:
xyChart:
plotColorPalette: "#E63946, #3B82F6, #059669"
---
xychart
title "Full Tree (🔴 PR | 🔵 main | 🟢 release)"
x-axis "max depth" [1, 2, 3, 4, 5, 6, 7, 8]
y-axis "time (ms)"
line [0.89, 1.93, 4.04, 6.20, 12.36, 24.95, 54.86, 110.71]
line [0.78, 1.94, 3.91, 6.24, 12.38, 25.38, 54.27, 112.00]
line [0.92, 1.83, 4.33, 6.68, 12.44, 24.50, 52.67, 108.52]
|
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
This PR adjusts TypeGPU’s resolution/error reporting to avoid implicitly resolving raw external JS arrays, while improving error stringification and updating tests to cover the corrected behavior.
Changes:
- Remove implicit resolution of raw JS arrays in
ResolutionCtxImpland adjust the thrown error message. - Improve
safeStringifyoutput for arrays. - Update/add tests to verify external array resolution via schema and to assert an error for untyped external arrays.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| packages/typegpu/tests/tgsl/wgslGenerator.test.ts | Updates inline snapshot to match new error message formatting. |
| packages/typegpu/tests/array.test.ts | Extends array tests to cover schema-wrapped external arrays and adds a regression test for untyped external arrays. |
| packages/typegpu/src/shared/stringify.ts | Adds explicit array formatting in safeStringify for clearer error messages. |
| packages/typegpu/src/resolutionCtx.ts | Removes raw array resolution and updates the thrown WgslTypeError message to use safeStringify. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if (Array.isArray(item)) { | ||
| return `[${item.map(safeStringify).join(', ')}]`; | ||
| } |
There was a problem hiding this comment.
safeStringify now recursively stringifies arrays without any cycle detection. Self-referential arrays (or arrays containing references that lead back to the same array) will cause infinite recursion / stack overflow while formatting an error message. Consider adding cycle detection (e.g., a WeakSet/Set of visited objects passed through recursion) and a fallback string (like "[<circular>]"), and optionally a max depth/length to keep error messages bounded.
6a465c3 to
c8434b3
Compare
No description provided.